fix(cross-runtime): close post-merge runtime review gaps by kojiwakayama · Pull Request #3543 · veryfront/veryfront-code

kojiwakayama · 2026-08-10T16:58:55Z

Summary

carry the still-unmerged review fixes from fix(cross-runtime): keep Node and Bun gates fail-loud #3537 onto current main
fail closed for Bun hosted config evaluation because Bun 1.3.6 does not enforce worker memory limits
harden Bun selection, workspace locks, npm link races, resolver coverage, and poisoned query parsing
add the requested boundary and replay regressions while preserving Node and Deno behavior

Verification

deno task test:node, full pass and natural exit
deno task test:bun, 1297 files passed, 0 failed
deno task test:unit, 3786 files and 28105 steps passed, 0 failed
deno task test:integration --no-lock, 283 files and 2726 steps passed, 0 failed
Linux Deno 2.7.7 deno task lint:ci
deno task typecheck
deno task audit
deno fmt --check
focused Node, Bun, and Deno regressions plus mutation checks

Context

PR #3537 merged before its final review-fix commit reached the branch. This PR contains only that post-merge delta, rebased onto current main.

Summary by CodeRabbit

Bug Fixes
- Improved handling when worker memory limits are unavailable, with clearer retryable errors.
- Improved Bun compatibility for hosted evaluation, extension loading, and workspace preparation.
- Improved request replay across retries and query parameter normalization.
- Added stricter HTTP port validation and cleaner server shutdown behavior.
Tests
- Expanded coverage for Bun execution, package linking, filesystem races, server ports, retries, and cache behavior.
Documentation
- Clarified YAML 1.1 fallback tag behavior under the YAML 1.2 core schema.

Code review exposed empty Bun selection, conservative lock ownership, symlink-race/cause handling, parser edge cases, poisoned iterators, and Bun's unenforced worker resourceLimits. The changes keep gates fail-closed while retaining Node/Deno hosted-worker coverage. Constraint: Bun 1.3.6 does not enforce node:worker_threads resourceLimits. Constraint: Missing or malformed workspace lock ownership is indeterminate and must be preserved. Rejected: Force-exit Node tests | hides leaked handles. Rejected: Reclaim every markerless Bun lock | races live lock initialization. Confidence: high Scope-risk: moderate Directive: Keep Node and Bun gates dependent on build:npm and fail closed when no tests are selected. Tested: deno task test:node (full pass, natural exit 0; dot reporter did not print an exact count). Tested: deno task test:bun (1297 files, 0 failed). Tested: deno task test:unit (3786 files, 28105 steps, 0 failed, 1 ignored). Tested: deno task test:integration --no-lock (283 files, 2726 steps, 0 failed after a single transient external esm.sh AbortError was confirmed by focused rerun). Tested: Linux Deno 2.7.7 deno task lint:ci. Tested: deno task typecheck. Tested: deno task audit. Tested: deno fmt --check. Tested: Focused Node, Bun, and Deno tests plus mutation checks for every new regression.

coderabbitai · 2026-08-10T16:59:08Z

📝 Walkthrough

Walkthrough

The PR adds Bun-specific worker memory-limit handling, updates Bun test infrastructure and workspace locking, renames the directory-link helper, and expands regression coverage for runtime adapters, request replay, cache normalization, imports, and YAML documentation.

Changes

Worker memory-limit handling

Layer / File(s)	Summary
Worker error contracts `src/config/declarative-evaluator.ts`, `src/config/declarative-evaluator-worker-protocol.ts`	The new `worker-memory-limit-unavailable` reason is added to evaluator and worker error validation.
Bun worker runner behavior `src/config/declarative-evaluator-worker-runner.ts`	Bun now rejects when bounded worker memory limits are unavailable. Worker-phase evaluation errors remain unchanged.
Worker behavior tests `src/config/declarative-evaluator-worker-runner.test.ts`	Tests verify the Bun rejection reason and skip hosted worker tests on Bun.

Bun test infrastructure

Layer / File(s)	Summary
Bun test selection and path handling `tests/bun/preload.ts`, `tests/bun/run-tests.mjs`, `tests/bun/runner-args.test.mjs`	The runner delays workspace setup until files are selected, validates missing-file filters by execution, and normalizes Windows and POSIX extension paths.
Workspace preparation and lock ownership `tests/bun/workspace-packages.mjs`, `tests/bun/workspace-packages.test.mjs`	Workspace preparation uses the shared link helper. Lock reclamation preserves locks with missing or invalid ownership data and rejects live or foreign owners.
Directory-link helper and error coverage `tests/ensure-npm-links.mjs`, `tests/ensure-npm-links.test.mjs`	The helper is renamed to `ensureDirectoryLink`. Tests cover concurrent races and filesystem error causes.

Runtime regression coverage

Layer / File(s)	Summary
Runtime and adapter regression tests `src/extensions/first-party-import.test.ts`, `src/modules/server/classify.test.ts`, `src/platform/adapters/runtime/node/http-server.test.ts`, `src/proxy/retry.test.ts`, `src/server/shared/renderer/adapter.test.ts`, `extensions/ext-yaml/src/adapter.ts`	Tests cover Bun resolver errors, encoded version markers, HTTP ports, multi-chunk retries, and hosted worker selection. YAML fallback-tag documentation is clarified.
Query normalization and cache identity `src/transforms/esm/http-cache-helpers.test.ts`	The cache test covers duplicate and encoded query parameters when array iteration and query append are unavailable.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Evaluator
  participant WorkerRunner
  participant Bun
  Evaluator->>WorkerRunner: evaluate configuration payload
  WorkerRunner->>Bun: detect runtime and worker capability
  Bun-->>WorkerRunner: bounded memory limit unavailable
  WorkerRunner-->>Evaluator: reject worker-memory-limit-unavailable

Possibly related PRs

veryfront/veryfront-code#3500: Both changes modify Bun test selection and runtime compatibility filtering.
veryfront/veryfront-code#3526: Both changes modify the YAML extension adapter.
veryfront/veryfront-code#3537: Both changes modify Bun worker handling and its tests.

Suggested reviewers: kwakayama

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately identifies a cross-runtime fix that addresses post-merge review gaps, although it does not specify the individual Bun and test-hardening changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/cross-runtime-review-followup

_{Comment @coderabbitai help to get the list of available commands.}

kojiwakayama · 2026-08-10T17:10:09Z

@coderabbitai review

coderabbitai · 2026-08-10T17:10:15Z

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

kojiwakayama · 2026-08-10T17:21:28Z

@coderabbitai review

coderabbitai · 2026-08-10T17:21:44Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/bun/preload.ts`:
- Around line 146-159: Add focused regression coverage for the preload
file-rewrite handler around the `posixPath` normalization and
extension/test-path filter, exercising both `/` and `\` separators for extension
source paths and test files. Verify both path styles trigger the expected
processing and preserve module contents.

In `@tests/bun/runner-args.test.mjs`:
- Around line 50-57: Update the spawnSync invocation in the “the Bun runner
fails loudly when filters select no files” test to import and use
fileURLToPath() when converting the run-tests.mjs file URL, replacing direct
URL.pathname usage while preserving the existing test arguments and behavior.

In `@tests/bun/workspace-packages.mjs`:
- Around line 51-71: The reclaimStalePreparationLock flow can delete a newly
recreated lock during concurrent stale-lock handling. Add an exclusive reclaimer
guard around the stale-lock replacement and new-marker write, revalidate
ownership/staleness after acquiring it, and release it reliably; extend the
existing tests with a two-process race regression confirming a live replacement
lock is not removed.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0afa3205-3773-49dc-8e2c-538201fe36f0

📥 Commits

Reviewing files that changed from the base of the PR and between 3842fa8 and 1c15f81.

📒 Files selected for processing (18)

extensions/ext-yaml/src/adapter.ts
src/config/declarative-evaluator-worker-protocol.ts
src/config/declarative-evaluator-worker-runner.test.ts
src/config/declarative-evaluator-worker-runner.ts
src/config/declarative-evaluator.ts
src/extensions/first-party-import.test.ts
src/modules/server/classify.test.ts
src/platform/adapters/runtime/node/http-server.test.ts
src/proxy/retry.test.ts
src/server/shared/renderer/adapter.test.ts
src/transforms/esm/http-cache-helpers.test.ts
tests/bun/preload.ts
tests/bun/run-tests.mjs
tests/bun/runner-args.test.mjs
tests/bun/workspace-packages.mjs
tests/bun/workspace-packages.test.mjs
tests/ensure-npm-links.mjs
tests/ensure-npm-links.test.mjs

kojiwakayama requested a review from kwakayama as a code owner August 10, 2026 16:58

kojiwakayama added this pull request to the merge queue Aug 10, 2026

coderabbitai Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread tests/bun/preload.ts

Comment thread tests/bun/runner-args.test.mjs

Comment thread tests/bun/workspace-packages.mjs

Merged via the queue into main with commit 80fb3a0 Aug 10, 2026
33 checks passed

kojiwakayama deleted the fix/cross-runtime-review-followup branch August 10, 2026 17:29

kojiwakayama mentioned this pull request Aug 10, 2026

fix(test): close remaining Bun review gaps #3548

Closed

kwakayama mentioned this pull request Aug 10, 2026

chore(release): v0.1.1228 #3550

Closed

kojiwakayama mentioned this pull request Aug 10, 2026

fix(bun): close late runtime review races #3551

Merged

kwakayama mentioned this pull request Aug 10, 2026

Release v0.1.1228 #3554

Merged

Navigation Menu

Uh oh!

fix(cross-runtime): close post-merge runtime review gaps - #3543

fix(cross-runtime): close post-merge runtime review gaps#3543
kojiwakayama merged 1 commit into
mainfrom
fix/cross-runtime-review-followup

kojiwakayama commented Aug 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

kojiwakayama commented Aug 10, 2026

Uh oh!

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

Uh oh!

kojiwakayama commented Aug 10, 2026

Uh oh!

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kojiwakayama commented Aug 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Context

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

kojiwakayama commented Aug 10, 2026

Uh oh!

coderabbitai Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kojiwakayama commented Aug 10, 2026

Uh oh!

coderabbitai Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

kojiwakayama commented Aug 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading